NW6| Fidaa Bashir| Cake co| Week 3 #184
Conversation
✅ Deploy Preview for cyf-module-html-css ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Ara225
left a comment
Ara225
left a comment
There was a problem hiding this comment.
Looks like a good first attempt, well done :) I've made some comments but I'm sure you're already aware of most. You're doing really really well with what you have done
| </p> | ||
| </section> | ||
| <section class="hero-sec"> | ||
| <img src="./img/background-img.webp" alt="vanilla chocolate cake" /> |
There was a problem hiding this comment.
Using good semantic html and some nice alt text
| height: 100px; | ||
| align-self: center; | ||
| } | ||
| header { |
There was a problem hiding this comment.
The header looks really good, good work
| pointer-events: none; | ||
| } | ||
|
|
||
| .menu-aside { |
There was a problem hiding this comment.
This needs some iteration to be more like the wireframe, hamburger menu on mobile and horizontail menu on desktop
| display: none; | ||
| } | ||
|
|
||
| .ham-label:has(input:checked) + .menu-aside { |
There was a problem hiding this comment.
Can see where you're going with this, good work 👍
| width: 100%; | ||
| height: 100%; | ||
| display: flex; | ||
| flex-direction: column; |
There was a problem hiding this comment.
This display would be what we want when the hamburger menu is open on mobile but it shouldn't be open by default and on desktop, it should be with the links next to eachother
| gap: var(--gap); | ||
| } | ||
|
|
||
| @media screen and (min-width: 640px) { |
There was a problem hiding this comment.
This is good - it's responsive and mobile first as requested by the intructions
2 participants
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.